Repository navigation
Convert float literals to wide decimal columns without InvalidOperation - #4372
rahulsmahadev wants to merge 1 commit into
Conversation
FloatLiteral.to(DecimalType) and DoubleLiteral.to(DecimalType) quantize the value under the default 28-digit decimal context, so a value whose quantized form has more digits raised decimal.InvalidOperation: amt > 1e26 on a decimal(38, 2) column, or amt < 0.5 on a decimal(38, 30) column. Quantize under a context with enough precision for the result. Co-authored-by: Isaac <no-reply@databricks.com>
|
@kevinjqliu @AP24110011695 could you take a look when you have a chance? It is the finite-value counterpart of #4287: float filters on wide decimal columns no longer raise |
AP24110011695
left a comment
AP24110011695
left a comment
There was a problem hiding this comment.
Verified failing-first and approving.
On main (fce8185): both cases raise decimal.InvalidOperation —
literal(1e26).to(DecimalType(38, 2))→ InvalidOperationliteral(0.5).to(DecimalType(38, 30))→ InvalidOperation
On this PR (f0f3520):
1e26 → (38, 2)=100000000000000004764729344.00— the exact binary float quantized at the column's scale, ROUND_HALF_UP as claimed0.5 → (38, 30)=0.500000000000000000000000000000- The
#4287contract is preserved:inf,-infandnanall still raise theValueError("value is not finite") from inside the helper tests/expressions/test_literals.pydecimal slice: 32 passed
The shared _float_to_decimal helper also removes the duplicated to(DecimalType) bodies in both literal classes — nice. The context precision (adjusted() + scale + 2) covers the rounding carry with margin to spare.
One non-blocking observation, no change requested: a literal whose quantized form exceeds 38 digits (e.g. 1e26 against decimal(38, 30) → 57 digits) now binds successfully and defers the failure to the scan path, where #4348's widened _literal_arrow_type hits decimal128's 38-digit cap as an ArrowInvalid. That's arguably the right deferral (the literal simply cannot exist in the column), but a clean bind-time ValueError for that case could be a small follow-up if you want bind/scan errors to line up.
Rationale for this change
A float filter on a wide decimal column raises
decimal.InvalidOperation:FloatLiteral.to(DecimalType)andDoubleLiteral.to(DecimalType)quantize the value to the column's scale under the default decimal context, which keeps 28 digits. When the quantized value has more digits than that (1e26with 2 decimals, or0.5with 30 decimals),quantizeraises. A decimal column can hold up to 38 digits, so these are valid filters.This change quantizes under a local context with enough precision for the result, in one helper used by both conversions. Rounding is unchanged (
ROUND_HALF_UP), and positive and negative infinity still raise theValueErroradded in #4287 for #4286. NaN remains rejected when a literal is constructed.Are these changes tested?
Yes.
test_float_to_decimal_conversion_beyond_default_context_precisionintests/expressions/test_literals.pyconverts2.0**87todecimal(38, 2)and0.5todecimal(38, 30)from both aFloatLiteraland aDoubleLiteral. It fails on main withdecimal.InvalidOperationand passes with this change.Are there any user-facing changes?
Bug fix: float filters on wide decimal columns no longer raise
decimal.InvalidOperation. No API changes.This pull request and its description were written by Isaac.